Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update adblock-rust to v0.8.1 #20113

Merged
merged 1 commit into from
Sep 14, 2023
Merged

Update adblock-rust to v0.8.1 #20113

merged 1 commit into from
Sep 14, 2023

Conversation

antonok-edm
Copy link
Collaborator

Resolves brave/brave-browser#32916

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

  • Add brave.com##+js(acs, this, probably, is, going, to, break, brave, and, crash, it, instead, of, ignoring, it) to the custom filters in brave://settings/shields/filters
  • Visit https://brave.com
  • The browser should not crash

@antonok-edm antonok-edm self-assigned this Sep 11, 2023
@antonok-edm antonok-edm requested review from a team and bridiver as code owners September 11, 2023 19:53
@github-actions github-actions bot added the CI/run-audit-deps Check for known npm/cargo vulnerabilities (audit_deps) label Sep 11, 2023
@brave-builds brave-builds mentioned this pull request Sep 11, 2023
Merged
7 tasks
brave-builds added a commit that referenced this pull request Sep 11, 2023
@brave-builds
Uplift of #20113 (squashed) to beta
77cff51
rillian
rillian approved these changes Sep 11, 2023
View reviewed changes
Copy link
Contributor

@rillian rillian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also update the version numbers in README.chromium and push a tag to the upstream adblock repo for easier comparison.

@antonok-edm antonok-edm merged commit b5298b4 into master Sep 14, 2023
14 checks passed
@antonok-edm antonok-edm deleted the adblock-rust-0.8.1 branch September 14, 2023 16:55
@github-actions github-actions bot added this to the 1.60.x - Nightly milestone Sep 14, 2023
kjozwiak pushed a commit that referenced this pull request Sep 14, 2023
@brave-builds
Update adblock-rust to v0.8.1 (uplift to 1.59.x) (#20114)
b3b63f6 
Uplift of #20113 (squashed) to beta
@kjozwiak
Copy link
Member

kjozwiak commented Sep 14, 2023 •
edited
Loading

Verification PASSED on Pixel 6 running Android 16 using the following build(s):

Brave | 1.60.34 Chromium: 117.0.5938.88 (Official Build) canary (64-bit)
--- | ---
Revision | 780ad58fcc8bc6a420bb0b5502669ccef653a907
OS | Android 14; Build/UPB5.230623.009; 34; REL

Using the STR/Cases outlined via #20113 (comment) with 1.57.62 Chromium: 116.0.5845.180, reproduced the original crash as per the following:

screen-20230914-151711.mp4

Because the Add Custom Filter feature has been removed from 1.60.x & 1.59.x due to the native UX/UI refactor, created https://gist.githubusercontent.com/kjozwiak/ad1f0681ac34cb2de3dc12f8c1d02be8/raw/46363fca9f528bb0166ca9ceef4af5f7c18ab93c/adblockVerification.txt to verify the issue using the STR/Cases outlined via #20113 (comment) running 1.60.34 Chromium: 117.0.5938.88 as per the following:

screen-20230918-234517.mp4

Verification PASSED on Win 11 x64 using the following build(s):

Brave | 1.60.33 Chromium: 117.0.5938.88 (Official Build) nightly (64-bit)
-- | --
Revision | 4dc51d292e9a72ef50db601670d8ef9379c7c96f
OS | Windows 11 Version 22H2 (Build 22621.2283)

Using the STR/Cases outlined via #20113 (comment) with 1.60.21 Chromium: 117.0.5938.62, reproduced the original crash as per the following:

Example Example
crash1 crash2

Using the same STR/Cases outlined via #20113 (comment), ensured that the crash doesn't occur with 1.60.33 Chromium: 117.0.5938.88 as per the following:

Example Example
fixed1 fixed2

@MadhaviSeelam MadhaviSeelam mentioned this pull request Sep 15, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bridiver bridiver bridiver approved these changes

@rillian rillian rillian approved these changes

@diracdeltas diracdeltas diracdeltas approved these changes

Assignees

@antonok-edm antonok-edm

Labels
CI/run-audit-deps Check for known npm/cargo vulnerabilities (audit_deps)
Projects
None yet
Milestone
1.60.x - Release
Development

Successfully merging this pull request may close these issues.

Crash when adding scriptlet injection filters with too many arguments
5 participants
@antonok-edm @kjozwiak @bridiver @rillian @diracdeltas